The GCLOUD_PROJECT_ID variable probably also is set in the web UI, just like GCLOUD_SERVICE_KEY, right? If so, add a comment about, please.

It seems to be used only in gcloud config set compute/zone GCLOUD_COMPUTE_ZONE so maybe just drop the variable?

You can keep it though, if this helps in you in writing a documentation.

Is this env var here necessary?

Still to be addressed.

it has already been addressed ... 😕

Yeah, in a separate place, where you dropped this env var in favor of just calling gcloud config set compute/zone europe-west1-b directly.

What I mean here is that this env var is not used in this step. The step is about pushing a docker image to some google docker registry and here the env var is not needed, right?

the env var sets the default compute zone for the project. I just saw that I was setting it again where I was pushing the docker image so I will be taking that out instead and keeping gcloud config set compute/zone europe-west1-b

Would setting IMAGE_NAME to eu.gcr.io/$GCLOUD_PROJECT_ID/$IMAGE_NAME-$CIRCLE_BUILD_NUM be better? I can see it could at least avoid creating a separate tag for the generated image and likely save you from repeating the image name for gcr in some places.

I did set it initially to eu.gcr.io/$GCLOUD_PROJECT_ID/habitat/habitat-operator-$CIRCLE_BUILD_NUM but the $CIRCLE_BUILD_NUM variable was not interpolated and I am not really sure why.

Would that thing work? https://circleci.com/docs/2.0/env-vars/#setting-an-environment-variable-in-a-shell-command In our case we could have something like the following as the very first step:

- run:
  name: Setup environment
  command: |
    echo 'export IMAGE_NAME=eu.gcr.io/$GCLOUD_PROJECT_ID/habitat/habitat-operator-$CIRCLE_BUILD_NUM' >>"${BASH_ENV}"
    echo 'export IMAGE_TAG=testing' >>"${BASH_ENV}"
    echo 'export IMAGE_FULL_NAME="${IMAGE_NAME}:${IMAGE_TAG}"' >>"${BASH_ENV}"

If that works, that it is a great way to avoid some repetitions (like repeating this long-winded docker image name or the "testing" tag in some places), and we could use it to tell make image to generate a docker image that contains the operator directly with the name we really want instead of creating the habitat-sh/habitat-operator image and then tagging it under something else. Also, you could then drop this part:

environment:
  IMAGE_NAME: habitat/habitat-operator

You could call make IMAGE=${IMAGE_NAME} TAG=testing image if you follow my suggestion for the IMAGE_NAME env var.

You could use ${IMAGE_NAME}:testing if you follow my suggestion about the IMAGE_NAME env var.

This step could be dropped if you follow my suggestion about the IMAGE_NAME env var.

Could you rename it to GKE_K8S_VERSION? I was a tiny bit confused if it doesn't clash with the K8S_VERSION env var from setup step.

You could use IMAGE_NAME if you followed yadda yadda.

You could use ${IMAGE_NAME}:testing if you followed my suggestion about the IMAGE_NAME env var.

Would that thing work? https://circleci.com/docs/2.0/env-vars/#setting-an-environment-variable-in-a-shell-command In our case we could have something like the following as the very first step:

- run:
  name: Setup environment
  command: |
    echo 'export IMAGE_NAME=eu.gcr.io/$GCLOUD_PROJECT_ID/habitat/habitat-operator-$CIRCLE_BUILD_NUM' >>"${BASH_ENV}"
    echo 'export IMAGE_TAG=testing' >>"${BASH_ENV}"
    echo 'export IMAGE_FULL_NAME="${IMAGE_NAME}:${IMAGE_TAG}"' >>"${BASH_ENV}"

If that works, that it is a great way to avoid some repetitions (like repeating this long-winded docker image name or the "testing" tag in some places), and we could use it to tell make image to generate a docker image that contains the operator directly with the name we really want instead of creating the habitat-sh/habitat-operator image and then tagging it under something else. Also, you could then drop this part:

environment:
  IMAGE_NAME: habitat/habitat-operator

Should --cluster-version receive ${GKE_K8S_VERSION} instead of $K8S_VERSION?

Still to be addressed.

Why don't you just drop the GetServiceIP function and change the WaitForLoadBalancerIP function to return (string, error)? You could do it with something like:

address := ""
err := wait.Poll(…, func() (bool, error) {
    …
    if len(…) == 0 {
        return false, nil
    }

    address = service.Status.LoadBalancer.Ingress[0].IP
    return true, nil
}
return address, err

Maybe you will want to return service.Spec.ClusterIP if wait.Poll returns an error, but not sure how useful it is.

we can drop service.Spec.ClusterIP because the test will still fail if it does not have the LoadBalancerIP itself

You could move this call to happen after time.Sleep(serviceStartupWaitTime), because right now we wait for the load balancer some time and then we unconditionally sleep for a fixed amount of time for the service to be ready. But the service is probably already ready by the time we get the load balancer IP. With the change, we can make the TestBind test a wee bit faster, because we will sleep for the service to be ready and maybe the load balancer will get the address at the same time and make the polling to last shorter.

Could you move this # GCLOUD_PROJECT_ID is set … comment to a separate line? Ideally above this one.

TAG=$IMAGE_TAG please.